Fix the build command passing -L/-l without links
authorAlex Crichton <alex@alexcrichton.com>
Tue, 4 Nov 2014 00:23:42 +0000 (16:23 -0800)
committerAlex Crichton <alex@alexcrichton.com>
Wed, 5 Nov 2014 19:37:34 +0000 (11:37 -0800)
src/cargo/ops/cargo_rustc/context.rs
src/cargo/ops/cargo_rustc/custom_build.rs
src/cargo/ops/cargo_rustc/mod.rs
tests/test_cargo_compile_custom_build.rs

index f6ca433fadca8cd16976548b1dbda8cdaabb9d7e..b2bd59eb2e185d821850c9300847be11d290214e 100644 (file)
@@ -1,7 +1,7 @@
 use std::collections::HashSet;
 use std::collections::hash_map::{HashMap, Occupied, Vacant};
 use std::str;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 
 use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target};
 use util::{mod, CargoResult, ChainError, internal, Config, profile};
@@ -9,6 +9,7 @@ use util::human;
 
 use super::{Kind, KindHost, KindTarget, Compilation, BuildOutput};
 use super::layout::{Layout, LayoutProxy};
+use super::custom_build::BuildState;
 
 #[deriving(Show)]
 pub enum PlatformRequirement {
@@ -22,7 +23,7 @@ pub struct Context<'a, 'b: 'a> {
     pub resolve: &'a Resolve,
     pub sources: &'a SourceMap<'b>,
     pub compilation: Compilation,
-    pub native_libs: Arc<Mutex<HashMap<String, BuildOutput>>>,
+    pub build_state: Arc<BuildState>,
 
     env: &'a str,
     host: Layout,
@@ -40,7 +41,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
                deps: &'a PackageSet, config: &'b Config<'b>,
                host: Layout, target: Option<Layout>,
                root_pkg: &Package,
-               native_libs: HashMap<String, BuildOutput>)
+               build_state: HashMap<String, BuildOutput>)
                -> CargoResult<Context<'a, 'b>> {
         let (target_dylib, target_exe) =
                 try!(Context::filename_parts(config.target()));
@@ -66,7 +67,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
             host_dylib: host_dylib,
             requirements: HashMap::new(),
             compilation: Compilation::new(root_pkg),
-            native_libs: Arc::new(Mutex::new(native_libs)),
+            build_state: Arc::new(BuildState::new(build_state, deps)),
         })
     }
 
index 727aebaec256789e6578d8b734414cebe7a49564..c11c64f6bbf9a1f8b4761207316fe0de3b796e5d 100644 (file)
@@ -1,9 +1,11 @@
+use std::collections::HashMap;
 use std::fmt;
 use std::io::fs::PathExtensions;
 use std::io::{fs, USER_RWX, File};
 use std::str;
+use std::sync::Mutex;
 
-use core::{Package, Target};
+use core::{Package, Target, PackageId, PackageSet};
 use util::{CargoResult, CargoError, human};
 use util::{internal, ChainError, Require};
 
@@ -22,6 +24,10 @@ pub struct BuildOutput {
     pub metadata: Vec<(String, String)>,
 }
 
+pub struct BuildState {
+    pub outputs: Mutex<HashMap<PackageId, BuildOutput>>,
+}
+
 /// Prepares a `Work` that executes the target as a custom build script.
 pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
                -> CargoResult<(Work, Work, Freshness)> {
@@ -40,7 +46,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
     // Start preparing the process to execute, starting out with some
     // environment variables.
     let profile = target.get_profile();
-    let mut p = super::process(to_exec, pkg, target, cx)
+    let mut p = try!(super::process(to_exec, pkg, target, cx))
                      .env("OUT_DIR", Some(&build_output))
                      .env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path()
                                                         .dir_path()
@@ -74,13 +80,15 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
             !t.get_profile().is_custom_build()
         }).unwrap();
         cx.dep_targets(pkg, non_build_target).iter().filter_map(|&(pkg, _)| {
-            pkg.get_manifest().get_links()
-        }).map(|s| s.to_string()).collect::<Vec<_>>()
+            pkg.get_manifest().get_links().map(|links| {
+                (links.to_string(), pkg.get_package_id().clone())
+            })
+        }).collect::<Vec<_>>()
     };
-    let lib_name = pkg.get_manifest().get_links().map(|s| s.to_string());
     let pkg_name = pkg.to_string();
-    let native_libs = cx.native_libs.clone();
-    let all = (lib_name.clone(), pkg_name.clone(), native_libs.clone(),
+    let build_state = cx.build_state.clone();
+    let id = pkg.get_package_id().clone();
+    let all = (id.clone(), pkg_name.clone(), build_state.clone(),
                script_output.clone(), old_build_output.clone(),
                build_output.clone());
 
@@ -109,11 +117,11 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
         // along to this custom build command.
         let mut p = p;
         {
-            let native_libs = native_libs.lock();
-            for dep in lib_deps.iter() {
-                for &(ref key, ref value) in (*native_libs)[*dep].metadata.iter() {
+            let build_state = build_state.outputs.lock();
+            for &(ref name, ref id) in lib_deps.iter() {
+                for &(ref key, ref value) in (*build_state)[*id].metadata.iter() {
                     p = p.env(format!("DEP_{}_{}",
-                                      super::envify(dep.as_slice()),
+                                      super::envify(name.as_slice()),
                                       super::envify(key.as_slice())).as_slice(),
                               Some(value.as_slice()));
                 }
@@ -139,10 +147,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
             human("build script output was not valid utf-8")
         }));
         let build_output = try!(BuildOutput::parse(output, pkg_name.as_slice()));
-        match lib_name {
-            Some(name) => assert!(native_libs.lock().insert(name, build_output)),
-            None => {}
-        }
+        build_state.outputs.lock().insert(id, build_output);
 
         try!(File::create(&script_output.join("output"))
                   .write_str(output).map_err(|e| {
@@ -166,7 +171,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
             try!(fingerprint::prepare_build_cmd(cx, pkg, Some(target)));
     let dirty = proc(tx: Sender<String>) { try!(work(tx.clone())); dirty(tx) };
     let fresh = proc(tx) {
-        let (lib_name, pkg_name, native_libs, script_output,
+        let (id, pkg_name, build_state, script_output,
              old_build_output, build_output) = all;
         let new_loc = script_output.join("output");
         try!(fs::rename(&old_script_output.join("output"), &new_loc));
@@ -177,10 +182,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
         let contents = try!(f.read_to_string());
         let output = try!(BuildOutput::parse(contents.as_slice(),
                                              pkg_name.as_slice()));
-        match lib_name {
-            Some(name) => assert!(native_libs.lock().insert(name, output)),
-            None => {}
-        }
+        build_state.outputs.lock().insert(id, output);
 
         fresh(tx)
     };
@@ -188,6 +190,27 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context)
     Ok((dirty, fresh, freshness))
 }
 
+impl BuildState {
+    pub fn new(overrides: HashMap<String, BuildOutput>,
+               packages: &PackageSet) -> BuildState {
+        let mut sources = HashMap::new();
+        for package in packages.iter() {
+            match package.get_manifest().get_links() {
+                Some(links) => {
+                    sources.insert(links.to_string(),
+                                   package.get_package_id().clone());
+                }
+                None => {}
+            }
+        }
+        let mut outputs = HashMap::new();
+        for (name, output) in overrides.into_iter() {
+            outputs.insert(sources[name].clone(), output);
+        }
+        BuildState { outputs: Mutex::new(outputs) }
+    }
+}
+
 impl BuildOutput {
     // Parses the output of a script.
     // The `pkg_name` is used for error messages.
index a3a9c83d50fe3a699cb1e474ee320640a3f656c7..be5dcf86e8afa5a78ce8023181ad6e2fa66963b5 100644 (file)
@@ -2,7 +2,6 @@ use std::collections::{HashSet, HashMap};
 use std::dynamic_lib::DynamicLibrary;
 use std::io::{fs, USER_RWX};
 use std::io::fs::PathExtensions;
-use std::os;
 
 use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve};
 use util::{mod, CargoResult, ProcessBuilder, CargoError, human, caused_human};
@@ -173,13 +172,9 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package,
         if target.get_profile().is_custom_build() {
             // Custom build commands that are for libs that are overridden are
             // skipped entirely
-            match pkg.get_manifest().get_links() {
-                Some(lib) => {
-                    if cx.native_libs.lock().contains_key_equiv(lib) {
-                        continue
-                    }
-                }
-                None => {}
+            if pkg.get_manifest().get_links().is_some() &&
+               cx.build_state.outputs.lock().contains_key(pkg.get_package_id()) {
+                continue
             }
             let (dirty, fresh, freshness) =
                     try!(custom_build::prepare(pkg, target, cx));
@@ -343,17 +338,20 @@ fn rustc(package: &Package, target: &Target,
         let rustc = if show_warnings {rustc} else {rustc.arg("-Awarnings")};
 
         // Prepare the native lib state (extra -L and -l flags)
-        let native_libs = cx.native_libs.clone();
+        let build_state = cx.build_state.clone();
         let mut native_lib_deps = Vec::new();
+        let current_id = package.get_package_id().clone();
+        let has_custom_build = package.get_targets().iter().any(|t| {
+            t.get_profile().is_custom_build()
+        });
 
         // FIXME: traverse build dependencies and add -L and -l for an
         // transitive build deps.
         if !target.get_profile().is_custom_build() {
             each_dep(package, cx, |dep| {
-                let primary = package.get_package_id() == dep.get_package_id();
-                match dep.get_manifest().get_links() {
-                    Some(name) => native_lib_deps.push((name.to_string(), primary)),
-                    None => {}
+                if dep.get_manifest().get_links().is_some() ||
+                   (*dep.get_package_id() == current_id && has_custom_build) {
+                    native_lib_deps.push(dep.get_package_id().clone());
                 }
             });
         }
@@ -364,13 +362,13 @@ fn rustc(package: &Package, target: &Target,
             // Only at runtime have we discovered what the extra -L and -l
             // arguments are for native libraries, so we process those here.
             {
-                let native_libs = native_libs.lock();
-                for &(ref lib, primary) in native_lib_deps.iter() {
-                    let output = &(*native_libs)[*lib];
+                let build_state = build_state.outputs.lock();
+                for id in native_lib_deps.iter() {
+                    let output = &(*build_state)[*id];
                     for path in output.library_paths.iter() {
                         rustc = rustc.arg("-L").arg(path);
                     }
-                    if primary {
+                    if *id == current_id {
                         for name in output.library_links.iter() {
                             rustc = rustc.arg("-l").arg(name.as_slice());
                         }
@@ -490,7 +488,7 @@ fn build_base_args(cx: &Context,
                          .rpath(root_profile.get_rpath())
     }
 
-    if profile.is_plugin() {
+    if profile.is_for_host() {
         cmd = cmd.arg("-C").arg("prefer-dynamic");
     }
 
index 52bd0b2e0fabf32000fdfab6f4296a207ba56fee..e83bbdadd7431f1cd55305a51b3a55963e2a8a61 100644 (file)
@@ -668,7 +668,8 @@ test!(build_cmd_with_a_build_cmd {
     --out-dir [..]target[..]deps --dep-info [..]fingerprint[..]dep-lib-a \
     -L [..]target[..]deps -L [..]target[..]deps`
 {compiling} foo v0.5.0 (file://[..])
-{running} `rustc build.rs --crate-name build-script-build --crate-type bin -g \
+{running} `rustc build.rs --crate-name build-script-build --crate-type bin \
+    -C prefer-dynamic -g \
     --out-dir [..]build[..]foo-[..] --dep-info [..]fingerprint[..]dep-[..] \
     -L [..]target -L [..]target[..]deps \
     --extern a=[..]liba-[..].rlib`
@@ -744,7 +745,7 @@ test!(output_separate_lines {
             }
         "#);
     assert_that(p.cargo_process("build").arg("-v"),
-                execs().with_status(0)
+                execs().with_status(101)
                        .with_stdout(format!("\
 {compiling} foo v0.5.0 (file://[..])
 {running} `rustc build.rs [..]`